Move Test Config to Jsonc#4297
Conversation
| public string DNSCachingConnString = null; | ||
| public string DNSCachingServerCR = null; // this is for the control ring | ||
| public string DNSCachingServerTR = null; // this is for the tenant ring | ||
| public bool IsAzureSynapse = false; // True for Azure Data Warehouse/Synapse |
There was a problem hiding this comment.
This was removed as it is not used.
| public string AliasName = null; | ||
|
|
||
| public static Config Load(string configPath = @"config.json") | ||
| public static Config Load(string configPath) |
There was a problem hiding this comment.
This was added as an escape hatch for preserving the behavior of ExtUtilities project. I have plans to remove the ExtUtilities project, so this method will hopefully go away someday.
| "WorkloadIdentityFederationServiceConnectionId": "", | ||
| "KerberosDomainPassword": "", | ||
| "KerberosDomainuser": "", | ||
| "IsManagedInstance": false |
There was a problem hiding this comment.
These fields exist in Config class, but did not in the json file.
| "SupportsIntegratedSecurity": true, | ||
| "LocalDbAppName": "", | ||
| "LocalDbSharedInstanceName": "", | ||
| "SupportsFileStream": false, |
There was a problem hiding this comment.
This field did not exist in the Config class.
| "DNSCachingServerTR": "", | ||
| "IsDNSCachingSupportedCR": false, | ||
| "IsDNSCachingSupportedTR": false, | ||
| "IsAzureSynapse": false, |
There was a problem hiding this comment.
This field existed in the Config class, but was removed since it's unused.
5885bb9 to
57e9b41
Compare
|
|
||
| ```bash | ||
| MDS_TEST_CONFIG=/path/to/config.json dotnet build -t:TestSqlClientManual -p:TestSet=2 | ||
| MDS_TEST_CONFIG=/path/to/config.jsonc dotnet build -t:TestSqlClientManual -p:TestSet=2 |
| <!-- Copy config file ================================================ --> | ||
| <Target Name="CopyConfig" BeforeTargets="Compile"> | ||
| <Copy SourceFiles="config.default.json" DestinationFiles="config.json" Condition="!Exists('config.json')" /> | ||
| <Copy SourceFiles="config.default.jsonc" DestinationFiles="config.jsonc" Condition="!Exists('config.jsonc')" /> |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4297 +/- ##
==========================================
- Coverage 66.57% 65.95% -0.63%
==========================================
Files 284 279 -5
Lines 43301 66219 +22918
==========================================
+ Hits 28827 43673 +14846
- Misses 14474 22546 +8072
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
paulmedynski
left a comment
There was a problem hiding this comment.
Looks great! I had updated VS Code to treat .json and .jsonc files as JSON with Comments, so I wasn't seeing any red squigglies :)
| type: boolean | ||
| default: false | ||
|
|
||
| - name: SupportsFileStream |
There was a problem hiding this comment.
This parameter is no unused and can be removed.
Description
As a byproduct of the PR pipeline, I got annoyed with the red squigglies under the comments in the test config.json file. Technically speaking jsonc files support comments while json does not. So, this small PR does the following:
#nullable🤖
Used 🤖 to find remaining references to config.json and replace with references to config.jsonc
Issues
N/A
Testing
Everything builds, but I want to validate with a PR run before taking it out of draft state.